libarchive: Always ensure we have a label#2452
libarchive: Always ensure we have a label#2452cgwalters wants to merge 1 commit intoostreedev:mainfrom
Conversation
The way SELinux works, there isn't a concept of "something without a label". There *is* the `unlabeled_t` type that the kernel makes up when it finds no label on a mounted filesystem. But that's still a label, it's just a kernel fallback. When we go to import a tar archive, in most cases it won't have SELinux labels, and even if it does, we want to overwrite it. Now, the concept of a "whiteout" file arose relatively recently as a special file type that's used in the container ecosystem alongside overlayfs. For some reason, presumably due to its special nature, at least the Fedora SELinux policy doesn't define a label for it. However, when it lands on disk, the kernel will still assign it a default label. And that's the problem - the computed checksum won't match, which will appear as corruption. One doesn't see this problem when e.g. extracting a tarball temporarily to disk, and then committing it because the act of extracting to disk will assign a label. This problem was hit when importing container layers for ostree-container work in ostree-rs-ext.
|
Reading on whiteout files, I found (https://www.kernel.org/doc/Documentation/filesystems/overlayfs.txt) which suggests that whiteout are 0:0 char devices. Still looking into the |
|
http://aufs.sourceforge.net/aufs.html > More reading for the |
| return FALSE; | ||
| if (!label) | ||
| { | ||
| if (!ostree_sepolicy_get_label (policy, "/usr/share/some-generic-thing", unix_mode, &label, cancellable, error)) |
There was a problem hiding this comment.
I think a more appropriate thing to do here would be to keep going up from relpath and stopping until we have a parent dir which has a label. This more closely matches the default heuristic used by SELinux.
There was a problem hiding this comment.
I'm not sure. In the case of this /var/tmp file, yes. On the other hand, if someone is trying to add /foobar, I don't think we want it to be root_t.
There was a problem hiding this comment.
I think it should be safe to trust the policy. E.g. /foobar would get caught by https://github.com/fedora-selinux/selinux-policy/blob/1715509773b3387a0c74423c05d53eb401d9b470/policy/modules/kernel/files.fc#L5.
| cancellable, error)) | ||
| return FALSE; | ||
|
|
||
| if (label) |
There was a problem hiding this comment.
Should become an assert now, I guess?
travier
left a comment
There was a problem hiding this comment.
Minor nit / question to make sure I understand the logic here but LGTM.
| return FALSE; | ||
| if (!label) | ||
| { | ||
| if (!ostree_sepolicy_get_label (policy, "/usr/share/some-generic-thing", unix_mode, &label, cancellable, error)) |
There was a problem hiding this comment.
Should we call this /usr/share/i-will-never-exist?
|
After a big of digging, the cause of this is likely And in practice, it is quite simply broken to have anything So, on one hand the answer could be "don't do that", and that's basically what ostreedev/ostree-rs-ext#112 is doing. Perhaps we should instead try to drive more opinions on this stuff into the ostree core. |
|
We can address this one in a different way. |
The way SELinux works, there isn't a concept of "something without
a label". There is the
unlabeled_ttype that the kernel makesup when it finds no label on a mounted filesystem. But that's still
a label, it's just a kernel fallback.
When we go to import a tar archive, in most cases it won't
have SELinux labels, and even if it does, we want to overwrite it.
Now, the concept of a "whiteout" file arose relatively recently
as a special file type that's used in the container ecosystem alongside
overlayfs.
For some reason, presumably due to its special nature, at least the
Fedora SELinux policy doesn't define a label for it.
However, when it lands on disk, the kernel will still assign it
a default label.
And that's the problem - the computed checksum won't match, which
will appear as corruption.
One doesn't see this problem when e.g. extracting a tarball
temporarily to disk, and then committing it because the act of extracting
to disk will assign a label.
This problem was hit when importing container layers for ostree-container
work in ostree-rs-ext.